BugFix3301: Precedence of % (mod) is higher than * and /#3311
BugFix3301: Precedence of % (mod) is higher than * and /#3311josdejong merged 4 commits intojosdejong:v14from
Conversation
josdejong
left a comment
There was a problem hiding this comment.
THanks @nkumawat34 , impressive! I thought this would be tricky to fix.
Two remarks:
- Can you update the Precedence table: https://github.com/josdejong/mathjs/blob/develop/docs/expressions/syntax.md#precedence
- Can you add more unit tests to check the precedence of
%andmodvs the four other operators*/.*and./? (precedence should be equal now)
|
Please check now I updated the PR |
|
Thanks, that's a thorough est of tests 😎 One idea: how about changing the function parseAndStringify(expr) {
return math.parse(expr).toString({parenthesis: 'all'})
}
// ...
assert.strictEqual(parseAndStringify('10 % 3 * 2'), '(10 % 3) * 2')Then there is no need for the explanatory comments like |
|
Thinking about it, we could do something like this: function assertParseAndEval(expr, expectedString, expectedResult) {
const node = parse(expr)
assert.strictEqual(node.toString({ parenthesis: 'all' }), expectedStr)
approxEqual(node.evaluate(), expectedResult)
}
// ...
assertParseAndEval('10 % 3 * 2', '(10 % 3) * 2', 2)😎 EDIT: I see there is already a helper function |
|
should I have to use parseAndStringifyWithParens or not ? |
|
I'm ok with any of the three options: leave as is, create and use assertParseAndEval, or use parseAndStringifyWithParens. I have a slight preference for using parseAndStringifyWithParens. What do you think? |
|
Ok I am going with this parseAndStringifyWithParens.I think it will be better. |
|
done |
|
Thanks for the quick updates 👌, merging your PR now in the |
|
Published now in |
Added the BugFix of Issue #3301
This PR addresses the problem where the precedence of the % (modulo) operator was incorrectly treated as having higher precedence than * and /. The fix ensures that % now has the same precedence as * and /, resolving the issue caused by the functional addition for % that led to incorrect operator precedence.